send queued messages in the background when the chat is not open - #41
Conversation
Previously an ordinary queued message only drained via useMessageQueue, which is mounted by the open ChatView. Leaving the chat unmounted the drain, so a queued message sat in the queue until the chat was reopened, even after the session became idle and ready. Extend the global released-queue drain into a general background queue drain that also claims ordinary queued heads when no foreground chat owns the session's queue: - Add a foreground queue ownership registry; useMessageQueue registers itself per session while mounted (and not read-only), and the background drain defers to any registered owner. - Rename useReleasedQueuedMessageDrain / ReleasedQueuedMessageDrain to useBackgroundQueuedMessageDrain / BackgroundQueuedMessageDrain and let it drain ordinary (non-berdctl, non-restored) heads for unowned sessions, including reacting to ownership release when the user leaves a chat. - Extract the berdctl cross-session origin check into a shared helper so the chat drains and the berdctl runtime use one definition. Restored-from-persistence heads and berdctl cross-session sends keep their existing behavior (session activation and the dedicated berdctl drain respectively). Session dispatch leases and queued-attempt ownership assertions still guarantee at most one active send attempt per message (LAWS/CHAT.md), and the queue now resumes sending when the session becomes ready regardless of which view is open.
1. Refresh reclaimed session queues from persistence before draining. When a session window closes, this renderer's in-memory queue copy is stale; draining from it could re-send a head the window already sent or dismissed (a duplicate user turn under LAWS/CHAT.md). Mirror the berdctl bridge's refreshReclaimedQueues so chat correctness does not depend on the optionally-loaded bridge: reload reclaimed queues from persistence (records arrive restored: true, which the drain refuses) before draining. 2. Track restored-head exclusions per record id. The restored flag is cleared by markQueuedMessagesReady during any session load, including loads the user did not initiate, which would let a relaunch-stale prompt fire. The drain now remembers restored heads it has seen and only lifts the exclusion when a foreground owner registers for the session - i.e., the user actually reopened the chat. 3. Surface background queued-send failures with an error toast. The background drain only claims unowned sessions, so a parked failure was invisible until the user reopened the chat. 4. Naming cleanup: rename the queue.releasedSendFailed i18n key to queue.backgroundSendFailed (drain-only usage) and update the stale 'released' test name in main.berdctl.test.ts. All fixes verified with discriminating tests (each fails against the pre-fix implementation).
morgmart
left a comment
There was a problem hiding this comment.
🤖 Automated code review
REQUEST_CHANGES. Static review of the exact three-dot range found one blocking queue-replay race and one non-blocking localization gap. The supplied GitHub evidence was inspected: all nine check runs for the exact head SHA completed successfully, while the combined commit status was pending with no legacy statuses. Final self-check covered the ordinary background-send and foreground handoff flows, detached-window ownership and reclaim, restored queues, berdctl-origin exclusion, contention and failure handling, navigation/consent guards, accessibility, i18n, async error/never-completes/lifecycle/race behavior, test honesty, and project laws/design-system rules. Findings were consolidated by underlying issue and have concrete changed-line evidence and user effects.
Deterministic publication result: 1 blocking and 1 non-blocking finding(s) publishable; 0 duplicate(s) suppressed.
morgmart
left a comment
There was a problem hiding this comment.
🤖 Re-reviewed at ce14a6e. The blocking overlapping-reclaim race is fixed through shared serialized reconciliation with regression coverage, and the Spanish failure-toast gap is closed with focused locale coverage. Local just check and the full 6,219-test suite pass.
* origin/main: polish home canvas widgets (#50) redesign shared and imported agent cards (#42) send queued messages in the background when the chat is not open (#41) # Conflicts: # src/features/home/onboarding/starterHomeLayout.ts # src/features/home/stores/homeWidgetRuntime.ts # src/features/home/stores/homeWidgetStore.test.ts # src/features/home/stores/homeWidgetStore.ts # src/features/home/ui/HomeView.tsx
…hat's first turn Starting a chat from the global composer could render the assistant reply above the user message with its text doubled. At promotion two drains woke at once: the chat store re-keys the queue to the backend id synchronously, but the mounted ChatView is still registered as the foreground queue owner under the renderer-local draft id until React commits the new `sessionId`. The background drain saw the promoted queue as unowned, claimed the head, and started `loadSessionMessages` — and because `loadingSessionIds` decides per notification whether a turn is replay, the prompt the remounted ChatView then dispatched had its whole live turn buffered as history, merged with the replayed chunks by `messageId`, and finally written over the transcript by `setMessages(sanitizeReplayMessages(buffer))`. Two guards, either sufficient alone: Honor the draft id as foreground ownership for a promoted session. A promoted row keeps its renderer-local id as `clientSessionId`, so the background drain now treats an owner registered under either id as the claimant and the hand-off instant can no longer look unowned. The foreground queue sends the head, as it did before PR #41 added this drain. Claim the dispatch target before hydrating, not after, in `acquireExistingSessionForBackgroundSend`. That publishes the window the load opens: any other sender — a ChatView the user opens mid-hydration, not just the promotion hand-off — sees contention and waits for the release instead of dispatching into a load that will misclassify its turn. Hydration under a held lease is already the pinned contract (`sessionActivation.test.ts` leases a session across a divergent load), so the target semantics are unchanged: a matching observation is absorbed and a divergent one is deferred to release. The lease is released on every failure path so a rejected load cannot leave a session looking busy. Conforms to LAWS/CHAT.md: a message MUST NOT have more than one active send attempt, and a session is ready only if it can accept a message — a session whose history load is in flight cannot accept one live. Still latent and out of scope: a prompt dispatched during a `session/load` that no lease covers (an activation load racing a foreground drain) would misroute the same way. Closing that needs queued-send readiness to gate on `loadingSessionIds`, which changes readiness semantics for berdctl's `--if-running` as well. Signed-off-by: Matt Toohey <contact@matttoohey.com>
Category: fix
User Impact: Queued messages now send automatically once the agent is free, even if you've navigated away from the chat.
Problem: Queuing a follow-up message and then leaving the chat silently stranded it — the queue only drained while the chat was open, so the message sat unsent until you happened to reopen that session. Solution: The always-mounted released-queue drain is generalized into a background queue drain: the open chat registers as the queue's foreground owner, and when it unmounts the background drain takes over and sends the head once the session is ready. The existing dispatch leases and ownership assertions still guarantee at most one send attempt per message (
LAWS/CHAT.md).File changes
src/features/chat/lib/foregroundQueueOwnership.ts (new)
Small registry where a mounted, interactive chat claims ownership of its session's queue. This is the coordination point that decides whether the foreground hook or the background drain sends a queued head — exactly one drain claims each head by design.
src/features/chat/hooks/useBackgroundQueuedMessageDrain.ts (renamed from useReleasedQueuedMessageDrain.ts)
The global drain now also claims ordinary queued heads when no foreground chat owns the session, and reacts when ownership is released (user leaves the chat). Hardened from review: reclaimed session-window queues are refreshed from persistence before draining (a stale in-memory copy can't re-send a head the window already sent or dismissed); restored-from-relaunch heads are excluded per record id until the user actually reopens the chat; and background failures raise an error toast since the user isn't looking at the failed session.
src/features/chat/hooks/useMessageQueue.ts
Registers as the foreground queue owner while mounted and not read-only. Its berdctl-origin check moved to the shared helper.
src/features/chat/lib/queuedMessageOrigin.ts (new)
Single shared definition of the berdctl cross-session origin check, previously duplicated in two modules. berdctl sends keep their dedicated drain and are excluded from both chat drains.
src/features/berdctl/commands/runtime/sessionSend.ts
Re-exports the shared origin check instead of defining its own copy.
src/features/chat/ui/BackgroundQueuedMessageDrain.tsx / src/features/chat/ui/ReleasedQueuedMessageDrain.tsx
Component renamed to match the hook's broader role; mount sites unchanged in behavior.
src/main.tsx / src/app/SessionWindowApp.tsx / src/app/main.berdctl.test.ts
Updated imports/assertions for the rename.
src/features/chat/hooks/useBackgroundQueuedMessageDrain.test.tsx
Existing released-head coverage kept; adds discriminating tests for foreground-owner deferral and handoff, window-reclaim persistence refresh, restored-head exclusion surviving incidental flag clears, and the failure toast. The two behavioral fixes were verified to fail against the pre-fix implementation.
src/shared/i18n/locales/en/chat.json
Renames
queue.releasedSendFailedtoqueue.backgroundSendFailed(single usage) and adds a title for the background failure toast.Reproduction Steps